Core Tools OOP Host #3802
Merged
Core Tools OOP Host #3802
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kshyju
reviewed
Sep 4, 2024
kshyju
reviewed
Sep 6, 2024
…tools into aibhandari/core-tools-oop
kshyju
reviewed
Sep 19, 2024
| { | ||
| FileName = inProc8FuncExecutablePath, | ||
| FileName = funcExecutablePath, |
There was a problem hiding this comment.
Where do we use the --no-build argument?
There was a problem hiding this comment.
We use it in line 626?
test/Azure.Functions.Cli.Tests/Azure.Functions.Cli.Tests.csproj
Outdated
Show resolved
Hide resolved
|
The table shared above doesn't seem to capture the expected supported scenarios. Shouldn't this reflect what is supported when
For inference, the path would always be:
|
5 tasks
fabiocav
reviewed
Sep 20, 2024
| @@ -418,15 +421,47 @@ private async Task<JObject> GetLocalSettingsJsonAsJObjectAsync() | |||
| public override async Task RunAsync() | |||
| { | |||
| await PreRunConditions(); | |||
| var isVerbose = VerboseLogging.HasValue && VerboseLogging.Value; | |||
There was a problem hiding this comment.
Can we move this logic that is specific to .NET to a different method and out of RunAsync? That will help declutter this method and make the code easier to navigate.
There was a problem hiding this comment.
This will also enable you to reduce the variables as you'll be able to drive the logic without needing to have an additional flag for OOP.
There was a problem hiding this comment.
Moved this logic into: ShouldExitAfterDeterminingHostRuntime to clean up RunAsync
fabiocav
approved these changes
Sep 24, 2024
kshyju
pushed a commit
that referenced
this pull request
Nov 4, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
aishwaryabh
added a commit
that referenced
this pull request
Nov 6, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
fabiocav
pushed a commit
that referenced
this pull request
Nov 6, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
aishwaryabh
added a commit
that referenced
this pull request
Nov 25, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
aishwaryabh
added a commit
that referenced
this pull request
Nov 25, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
aishwaryabh
added a commit
that referenced
this pull request
Nov 25, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
aishwaryabh
added a commit
that referenced
this pull request
Nov 26, 2024
* this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement
aishwaryabh
added a commit
that referenced
this pull request
Jan 28, 2025
* Core Tools OOP Host (#3802) * this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * removing extra line in node * addressing comments * moving validate host runtime to its own method * forgot to add return statement * Custom host changes for VS Scenario (#3831) * this doesn't work * this finally works now * default should be oop host * added edge cases * fixing formatting * using determineTargetFramework * saving tests * adding copy step for OOP * fixing the webhost reference * removing test * setting host version in ps script * update to validate worker versions script * update versions for worker packages * adding ToString * validating worker version * validate worker versions * updating csproj to compile * addressing pr feedback * updating build steps * adding build step * fixing build step * trying to get this working * reverrting target runtimes * updating tests * adding dotnet info step * adding changes * removing extra test * trying to specify architecture * modifying tests to see if they work * narrowing down to tests that are failing * trying to see if it works with nobuild flag * addressing pr feedback * updating tests with latest logging * addressing comments and marking flaky tests * updating so that we are only using net8 framework * pushing change for branch build * adding single quotes * reverting quotes * adding code mirror fiile * updating build step * updatinng build steps * updating build step * adding step for dotnet publish * set inprochost compilation system to diff value and skip flaky test * updating public build pipeline to trigger * adding extra changes for pipeline * public build yml * updating official build * readd net8 build artifact step * readding space back * addressing initial comments * adding explicit openTelemetry dlls * simplifying logic of startHostAction * addressing PR feedback * start tests * changing some of the tests back * reverting test back to normal * fixing spacing for csproj * addressing PR feedback * adding extra variable * adding logic for edge case scenarios * added edge cases tests * initial changes * removing extra line in node * removing files not needed for custom host * adding build steps * addressing comments * moving validate host runtime to its own method * forgot to add return statement * trying out creating directory * remove skip artifact gen * updating build step * updating build to include in official pipeline * updating publishing custom host step * code mirror * trying to get official build working * program cs update * updating custom host to work with parsing local.settings.json * console read line for stalling process * removing changes not needed for custom host * only including inproc6 and inproc8 changes * addressing initial comments * removing skip artifact gen flag * verbose logging * tryna add release pipeline * windows yaml changes * addressing comments about yml * updating log message * fixing spacing * fixing spacing again * trying spacing again * spacing change * tryan fix spacing this way * spacing * spacing * new line * regular indentation * super indent * changing spacing back to normal * addressing comments * fixing pipeline * Fix build to remove uploadToStorage (#3839) * build steps * trying to parallelize build steps * Revert "build steps" This reverts commit 9aa82fa. * Reapply "build steps" This reverts commit a7b7d46. * Revert "trying to parallelize build steps" This reverts commit d10ef5c. * Adding artifact assembler tool which consolidates the artifacts and produce the package (#3843) * Adding artifact assembler program which collects artifacts from different sources and assemble the final package for core tools. * Adding README * Remove downloaded artifact directories. Minor language update to README * Updating the artifact name to include "_inproc" suffix (#3849) * Small refactor and new tests (#3835) * Updating coretools to detect unsupported inproc6 sdk scenario (#3825) * Restructuring custom host to improve error handling (#3861) * Create Artifacts for releasing OOP core tools (#3848) * assemble artifacts * adding code mirror to test * try awaiting tasks to save space * removing --net8 dependency * trying to delete directory one at a time * trying again with extracting * diff build steps for vs and cli * forgot to include other step for var * trying out new changes * clean up each zip file after extracting * trying another approach * try to delete each directory after copying * removing logic for creating zips rn * including e2e test changes and add powershell file * removing multi targeting * pass in staging directory * adding param * adding logging * printing out paths * adding isolated e2e tests and changing verbosity for logger * adding a new label * comments * adding more clarifying comments * removing extra test and types * adding logging statement for failing task if we fail tests * updating tests to only run in winx64 and winx86 * target directory * target directory * changes to msi file * print out directory * resolving path * reorg files and try to set env variable * dont delete artifacts * update generate msi files * add doller sign for variable name and fix e2e test * changing runtime version * updating to get correct cli version * lol forgot to have content in zip ps script * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * Update src/Azure.Functions.ArtifactAssembler/ArtifactAssembler.cs Co-authored-by: Lilian Kasem <[email protected]> * addressing comments * reverting programcs back to the way it was * staging directory not correct * merging * Revert "merging" This reverts commit 89cc110. * fixing spacing * wrong directory * trying to free up disk space * removing custom code mirror * removing dollar signs * updating build number * adding variables back * fix build number * fixing candle * fixing msi files * reverting files back to normal --------- Co-authored-by: Lilian Kasem <[email protected]> * Adding package reference (#3872) * adding system.text.json * update patch * move to 8.0.10 * fixing indentation * trying to change version * Templates version update (#3883) * Create metadata.json file in conslidated artifacts intermediate pipeline (#3884) * adding generate metadata file * remove code mirror * get correct inproc version * print out metadata file * fixing zip issue * fixing release number * fixing release number again * removing zip artifact script and fixing code mirror * template update (#4151) * Update templates for dotnet isolated worker 2.x (#4189) * adding tests for custom host * removing zipping for now * trying to print out value * update artifact assembler * removing extra delete file lol * adding env variables * add language * trying to see if this works for e2e tests * this should work * setting language * trying out new change * trying to set env variable * updating start tests * setting env variable * updating cli tester * trying to set testVsArtifacts * setting paths back to normal * local settings json parser print current directory * adding local.settings.json * tests should fail now if they actually fail * adding net6 inproc tests * updating path * start tests net6 update * updating zipping to happen * updating test project * fixing e2e test project * add trailing comma and test comment * updating tests to not exit with expected error * adding tests for custom host * removing zipping for now * trying to print out value * update artifact assembler * removing extra delete file lol * adding env variables * add language * trying to see if this works for e2e tests * this should work * setting language * trying out new change * trying to set env variable * updating start tests * setting env variable * updating cli tester * trying to set testVsArtifacts * setting paths back to normal * local settings json parser print current directory * adding local.settings.json * tests should fail now if they actually fail * adding net6 inproc tests * updating path * start tests net6 update * updating zipping to happen * updating test project * fixing e2e test project * add trailing comma and test comment * updating tests to not exit with expected error * updating artifact assembler * forgot to update assembler * changing environment variable back to normal * addressing comments * addressing comments * deleting extra test csproj folder entry and adding comments to pipeline helpers * adding osx support (#4213) * removing duplicate code * using net8 binaries instead of net6 for custom host osx * addressing feedback for method name --------- Co-authored-by: Shyju Krishnankutty <[email protected]> Co-authored-by: Fabio Cavalcante <[email protected]> Co-authored-by: Surbhi Gupta <[email protected]> Co-authored-by: Matthew Henderson <[email protected]> Co-authored-by: Lilian Kasem <[email protected]> Co-authored-by: Jacob Viau <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue describing the changes in this PR
resolves #3744
This PR creates the feature branch
feature/oop-host, which has the OOP host build. This PR also includes checks for the user passing in the--runtimeparam specifying the host and infers the host if that is not specified. I've also added tests which checks that we try to execute the correct func.exe (even though those tests technically fail since there is noinproc-6orinproc-8folder). We also only run on .NET 8 TFM and remove multi-targeting in the feature branch.For when the user specifies the
--runtimeparameter, these are the following scenarios we support today where the first row is the options the user may put in and the first column are the potential apps a user may have:Anything that does not have an X within it throws an error to the user with a helpful message on why the passed in host runtime is not valid.
For inference the path will be:
This is how the logic looks like today for the inference scenario:
FUNCTIONS_INPROC_NET8_ENABLEDis set- Run inproc8 host
- Run inproc6 host
Pull request checklist